sql: add ALTER TABLE ... SET SCHEMA to the declerative schema changer #158141
Conversation
Previously, namespace had descID, databaseID, and name as attributes. DatabaseID was replaced with schemaID to support `ALTER TABLE ... SET SCHEMA`. Epic CRDB-31281 Release note: None
|
Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
This change adds support for `ALTER TABLE ... SET SCHEMA` to the declarative schema changer. Before, this operation was ran by the legacy schema changer. Epic CRDB-31281 Fixes cockroachdb#155989 Release note (sql change): `ALTER TABLE ... SET SCHEMA` is supported by the declerative schema changer.
shghasemi
force-pushed
the
set-schema
branch
from
68c8641 to
cd6cc7a
Compare
fqazi
left a comment
fqazi
left a comment
There was a problem hiding this comment.
@shghasemi Great work!
Some minor comments here
@fqazi reviewed 113 of 113 files at r1, 17 of 17 files at r2, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @msbutler)
pkg/sql/schemachanger/scbuild/internal/scbuildstmt/rename_table.go line 108 at r2 (raw file):
func getAlterTableQualifiedObjectName(name *tree.UnresolvedObjectName, b BuildCtx) tree.TableName { objectName := name.ToTableName() dbElts, scElts := b.ResolveTargetObject(name, privilege.CREATE /* this should be 0 */)
We could use b.NamePrefix and just take in the element instead, the prefix is already resolved for us when resolved the relation.
pkg/sql/schemachanger/scbuild/internal/scbuildstmt/rename_table.go line 108 at r2 (raw file):
func getAlterTableQualifiedObjectName(name *tree.UnresolvedObjectName, b BuildCtx) tree.TableName { objectName := name.ToTableName() dbElts, scElts := b.ResolveTargetObject(name, privilege.CREATE /* this should be 0 */)
Also do we need a parameter for the privilege here?
pkg/sql/schemachanger/scbuild/internal/scbuildstmt/rename_table.go line 178 at r2 (raw file):
} func getAlterTableTargetElement(
Let's call this getRelationElement.
pkg/sql/schemachanger/scplan/testdata/create_schema line 7 at r1 (raw file):
transitions: [[Schema:{DescID: 104}, PUBLIC], ABSENT] -> PUBLIC [[Namespace:{DescID: 104, Name: sc, ReferencedDescID: 0}, PUBLIC], ABSENT] -> PUBLIC
Nit: This is mildly annoying but its harmless since the descriptor ID is always unique when creating objects.
Temporary commit to fix review comments and test fails. This commit is added to make review easier. Will be squashed after all work is done. Epic CRDB-31281 Release note: None
3 participants
This change adds support for
ALTER TABLE ... SET SCHEMAto thedeclarative schema changer. Before, this operation was ran by the
legacy schema changer.
Epic CRDB-31281
Fixes #155989
Release note (sql change):
ALTER TABLE ... SET SCHEMAis supported bythe declerative schema changer.